Skip to content

Conversation

@webwarrior-ws
Copy link
Contributor

Added new NoAsyncRunSynchronouslyInLibrary rule and tests for it.

@webwarrior-ws webwarrior-ws force-pushed the no-run-synchronously branch 4 times, most recently from 8fbe79e to 43fa62a Compare November 18, 2025 11:39
@knocte
Copy link
Collaborator

knocte commented Nov 18, 2025

Make rule pass new tests by checking all nodes in the file for test attributes

But we don't want the check to be just about the file but about the whole assembly.

@webwarrior-ws
Copy link
Contributor Author

Make rule pass new tests by checking all nodes in the file for test attributes

But we don't want the check to be just about the file but about the whole assembly.

The whole assembly check will only work when linting project or solution.

@knocte
Copy link
Collaborator

knocte commented Nov 18, 2025

The whole assembly check will only work when linting project or solution.

I know. That's why we don't recommend (show a warning) when only linting files.

@knocte
Copy link
Collaborator

knocte commented Nov 18, 2025

BTW, in my comment (#785 (comment)) I'm quoting some text from a commit msg, that either should be fixed (changed the wording), or implemented properly.

@knocte
Copy link
Collaborator

knocte commented Nov 18, 2025

BTW, in my comment (#785 (comment)) I'm quoting some text from a commit msg, that either should be fixed (changed the wording), or implemented properly.

After you have fixed the above, let's add 2 more commits:

  • 1st will be, in all cases where you have changed the API of FSharpLint.Core in this PR (for example to change Foo(): Bar to AsyncFoo(): Async<Bar>), reintroduce the old method/function: Foo() that will simply call Async.RunSyncronously AsyncFoo(). CI of this commit will obviously fail on SelfCheck.
  • 2nd commit will decorate all these methods with an Obsolete attrib, will also change the rule to ignore methods/functions with this attrib, add 2 more tests, and its CI should be green.

@webwarrior-ws webwarrior-ws force-pushed the no-run-synchronously branch 3 times, most recently from 75e8b61 to b83aebd Compare November 19, 2025 08:35
@webwarrior-ws webwarrior-ws force-pushed the no-run-synchronously branch 2 times, most recently from b49d71b to baecfb8 Compare November 19, 2025 10:44
@webwarrior-ws webwarrior-ws force-pushed the no-run-synchronously branch 2 times, most recently from a2618d9 to 3dd6e70 Compare November 19, 2025 12:00
No checks for assembly name yet.
Check assembly name. If it contains "test" substring, assume
this is test project and don't fire the rule.
For NoAsyncRunSynchronouslyInLibrary rule that make sure that
code inside NUnit and  MSTest tests doesn't trigger the rule.

Includes failing tests.
Check if Async.RunSynchronously call is iniside NUnit or MSTest
tests. If it is, don't trigger the rule.
Add configuration for the rule and enable it by default.
Assembly.QualifiedName is empty, so instead check namespace and
project name.
Simplified check for entry point.

Also combined some code that uses args.CheckInfo to not pattern
match on it twice.
That also calls `Async.RunSynchronously` and causes CI to fail.
Got rid of Async.RunSynchronously calls in FSharpLint.Core.
Suppress NoAsyncRunSynchronouslyInLibrary rule.
Exclude projects that have "console" in their name addition to
those that contain "test".
Updated docs for NoAsyncRunSynchronouslyInLibrary to make clear
what code is considered to be library code.
When linting project. This will give more information for rules
to use, such as all types defined in an assembly.
For EntryPoint point attribute instead of just current file.
This reverts commit 7ba5ca4.
Suppression is no longer needed because Benchmarks project has
`[<EntryPoint>]` attribute which is now properly recognized.
Added 2 more tests for NoAsyncRunSynchronouslyInLibrary rule.
Make rule pass new tests by checking all nodes in the file for
test attributes, not only parent nodes.

Also when checking a project, check all files in the project
for classes that are marked by test attributes.
That were removed in commit 9de63ea ("Core,Console,Tests: refactoring").
Methods and functions when applying the rule.

Added 2 more tests.

Marked non-async parsing methods re-introduced in previous
commit with `[<Obsolete>]` attribute.
@webwarrior-ws
Copy link
Contributor Author

Rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants